Self-signed certificate support by jreidinger · Pull Request #2270 · agama-project/agama

jreidinger · 2025-04-15T14:31:32Z

Problem

Using registration server that uses self-signed certificate is not possible.

Solution

In this initial pull request implement question for user if certificate should be imported or not.

Testing

Added a new unit test
Tested manually

json used to invoke popup:

{
  "product": {
    "id": "SLES",
    "registrationCode": "test",
    "registrationUrl": "https://migration-rmt2.qe.nue2.suse.org"
  }
}

json to test unattended ( will return product not found on server instead of certificate product as I do not have any rmt with SLES16 support)

{
  "product": {
    "id": "SLES",
    "registrationCode": "test",
    "registrationUrl": "https://migration-rmt2.qe.nue2.suse.org"
  },
  "security": {
	  "sslCertificates": [{
		 "fingerprint": "F6:7A:ED:BB:BC:94:CF:55:9D:B3:BA:74:7A:87:05:EF:67:4E:C2:DB",
		"algorithm": "SHA1"
	  }
	 ]
  }
}

Screenshots

Documentation

Remember to look at it from the user's perspective. Yes you have made the compiler happy.
But will the humans even know about your contribution? Sometimes they cannot miss it,
other times they need advertisement and explanation.

Look for relevant sections and adjust:

https://agama-project.github.io/ (source)
Run: git ls-files '*.md'

jreidinger · 2025-04-22T15:44:20Z

@joseivanlopez thanks, changed popup looks really good. So now only rest of backend part for unattended support is missing.

dgdavid · 2025-04-23T05:40:46Z

+      <Popup.Actions>
+        <QuestionActions
+          actions={question.options}
+          defaultAction={question.defaultOption}


NP: Probably not in the scope of this PR, but reveleaded here.

I believe we have to look for a way for not having a "primary" action but just the focused one for prompts like this where Agama shouldn't use a call to action. Agama should just prevent users trusting in the self certificate by accident focusing the Reject by default, but not encouraging/inviting users to reject it by using a primary action look&feel.

imobachgs

I have reviewed the Rust part.

imobachgs · 2025-04-23T12:43:55Z


+    pub async fn set_registration_url(&self, url: &String) -> Result<(), ServiceError> {
+        self.client
+            .put_void("/software/registration/url", url)


It looks weird to me to have separate endpoints to set the registration code/email and the URL.

Perhaps in the future we might consider to add the URL to the registration params.

yeap, but code+email is together registration...not sure if it should be also part of registration method param and expect that all following registration of addons is forced to be on identical url.

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

imobachgs

Apart from two minor comments, the Rust part is looking good.

imobachgs · 2025-04-24T06:54:27Z

                Ok(SSLFingerprint {
                    fingerprint: value,
-                    algorithm: alg.try_into()?,
+                    algorithm: super::model::SSLFingerprintAlgorithm::from_str(&alg)?,


I would prefer to import the SSLFingerprintAlgorithm as you did with SSLFingerprint and SecuritryProxy.

imobachgs · 2025-04-24T06:55:35Z

        let dbus_list: Vec<(&str, &str)> = list
            .iter()
-            .map(|s| (s.algorithm.to_str(), s.fingerprint.as_str()))
+            .map(|s| (s.algorithm.clone().into(), s.fingerprint.as_str()))


You can save the explicit clone if you derive Copy. In an enum, it makes sense.

imobachgs · 2025-04-24T14:05:12Z

+
+      def self.download(url, insecure: false)
+        # TODO
+        # result = Downloader.download(url, insecure: insecure)


Is something missing here?

no, it is just for future download of certificate from URL if we decide to implement it. It is using Yast downloader, so I kind of expect that we will do it differently in agama.

imobachgs

Please, ~~add a reference to this PR and~~ you can merge :-)

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

The Rust part of the project was introduced just as a command-line interface. However, it has turned into a full blown HTTP/API server (with a command-line) and we plan to move more code from Ruby to Rust. We are learning Rust as we go, so some old decisions might need to be revisited. I have started by trying to simplify the [ServiceError](https://github.com/agama-project/agama/blob/70e7d987c9478f1c0ed3d1bf0c851485b9b88d84/rust/agama-lib/src/error.rs#L29-L78) enum, which represents an error but includes variants for many different situation (even unrelated ones). We even have an "anyhow" variant! This first round includes: - Defining an error for each HTTP client. - Defining an error for each store. - Defining an error for the storage, which includes one variant for each store. - Remove some `ServiceError` variants. - Make use of `anyhow` in more places of the binary. Once we have put everything into a better place, we can decide to simplify the whole thing if needed. Let's go step by step. ## To do - [x] Test the changes - [ ] Convert errors with a single variant into structs (?) - [x] Adapt changes from #2270. - [x] Update the changes file ## Follow-up - There is another whole category errors that are related to D-Bus. Let's move them to their own error and finish the `ServiceError` clean-up. - The HTTP client constructors do not need to be async. - The D-Bus clients are the "special cases". Perhaps we should consider moving from DomainHTTPClient to DomainClient (and DomainClient to DomainDBusClient). > [!WARNING] > Do not merge before #2270. We need to fix some conflicts first.

## Problem We recently implemented support for specifying a certificate to be used during registration. See #2270. Conversion of the fingerprints in an AutoYaST profile to the new Agama configuration format is still missing. ## Solution Implement the conversion of AutoYaST `reg_server_cert_fingerprint` and `reg_server_cert_fingerprint_type` to the Agama format: ```json { "security": { "sslCertificates": [ { "fingerprint": "01:12:34:45:56:67:78:89:90", "algorithm": "SHA1" } ] } } ``` ## Testing Added new unit tests

Prepare to release Agama 15: * #2258 * #2270 * #2277 * #2279 * #2283 * #2284 * #2285 * #2286 * #2287 * #2288 * #2291 * #2292 * #2293 * #2295 * #2297 * #2299 * #2300 * #2301 * #2302 * #2303 * #2305 * #2306 * #2307 * #2308 * #2309 * #2313 * #2314 * #2315 * #2317 * #2318 * #2319 * #2320 * #2321 * #2322 * #2323 * #2324 * #2325 * #2328 * #2329 * #2330 * #2331 * #2335 * #2336 * #2337 * #2338 * #2339 * #2340 * #2342 * #2345 * #2346 * #2348 * #2349 * #2350 * #2351 * #2352 * #2353 * #2354 * #2355 * #2357 * #2358 * #2359 * #2360 * #2361 * #2362 * #2363 * #2364 * #2365 * #2366 * #2368 * #2369 * #2370 * #2371 * #2372 * #2374 * #2377 * #2378 * #2379 * #2380 * #2381 * #2382 * #2384 * #2385 * #2386 * #2388 * #2389 * #2390 * #2391 * #2392 * #2394 * #2397 * #2398 * #2401 * #2403

jreidinger and others added 5 commits April 15, 2025 16:16

Implement initial question to import certificate

67d2585

Merge remote-tracking branch 'origin/master' into initial_certificates

31b1e37

experiment with paragraphs

72873ff

add fingerprint storage and use it in registration

abf9ff0

service: add data to certificate question

65ceb8d

jreidinger commented Apr 16, 2025

View reviewed changes

Comment thread service/lib/agama/registration.rb Outdated

jreidinger added 5 commits April 16, 2025 21:36

allow registration url be redefined and also add ssl fingerprints

5946bbb

update dbus doc and fix typo

930f816

add proxies for security

a57e474

Add registration url to rust part

7b65d88

implement for security in rust part that communicate with dbus

6901bae

ancorgs mentioned this pull request Apr 21, 2025

AutoYaST conversion of registration fingerprint #2277

Merged

joseivanlopez added 2 commits April 22, 2025 14:26

service: fix registration certificate question

48e5991

web: add question for registration certificate

4cb3cf9

jreidinger added 2 commits April 22, 2025 19:52

Merge remote-tracking branch 'origin/master' into initial_certificates

41663b2

add web part of security

90d795d

dgdavid reviewed Apr 23, 2025

View reviewed changes

jreidinger added 5 commits April 23, 2025 13:21

rename key in profile

d8c0b16

apply clippy suggestion

9354ceb

make rubocop happy

f1b0180

fix rust unit test

6d0d98a

adjust dbus doc to make ci happy

64ba358

jreidinger marked this pull request as ready for review April 23, 2025 13:00

imobachgs reviewed Apr 23, 2025

View reviewed changes

jreidinger and others added 2 commits April 23, 2025 18:12

Apply suggestions from code review

9214f64

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

changes from code review

caa3559

imobachgs reviewed Apr 24, 2025

View reviewed changes

jreidinger added 2 commits April 24, 2025 09:11

changes from review

6696dee

cargo fmt

22a03c9

imobachgs mentioned this pull request Apr 24, 2025

refactor(rust): reduce ServiceError (round 1) #2292

Merged

4 tasks

imobachgs approved these changes Apr 24, 2025

View reviewed changes

changes

6eb69ae

imobachgs approved these changes Apr 24, 2025

View reviewed changes

Comment thread rust/package/agama.changes Outdated

Comment thread rust/package/agama.changes Outdated

Apply suggestions from code review

2fa3f3d

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

jreidinger merged commit cd9efcf into master Apr 24, 2025
11 checks passed

jreidinger deleted the initial_certificates branch April 24, 2025 14:32

imobachgs mentioned this pull request May 26, 2025

Update to release 15 #2405

Merged

Conversation

jreidinger commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Testing

Screenshots

Documentation

Uh oh!

Uh oh!

jreidinger commented Apr 22, 2025

Uh oh!

dgdavid Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imobachgs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

imobachgs Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

jreidinger Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

imobachgs left a comment

Choose a reason for hiding this comment

Uh oh!

imobachgs Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

imobachgs Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

imobachgs Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

jreidinger Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

imobachgs left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jreidinger commented Apr 15, 2025 •

edited

Loading

dgdavid Apr 23, 2025 •

edited

Loading

imobachgs left a comment •

edited

Loading